refactor(nav-drawer): Use native dialog for non-relative nav-drawers#2194
refactor(nav-drawer): Use native dialog for non-relative nav-drawers#2194rkaraivanov wants to merge 16 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors igc-nav-drawer to use the native <dialog> element for non-relative drawers (top-layer rendering) and adds user-interaction close events, aligning with the goal of avoiding z-index/stacking issues and enabling external consumers to react to user-driven closes.
Changes:
- Switch non-relative nav-drawer rendering/behavior to a native modal
<dialog>and addkeepOpenOnEscape. - Emit
igcClosing(cancelable) andigcClosedfor user-interaction closes; export the component event map type. - Update Storybook and unit tests to cover dialog-based behavior and the new close events.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| stories/nav-drawer.stories.ts | Adds Storybook action handlers and a keepOpenOnEscape control; updates drawer item content markup. |
| src/index.ts | Exports IgcNavDrawerComponentEventMap from the package entrypoint. |
| src/components/nav-drawer/nav-drawer.ts | Refactors non-relative mode to native <dialog>, adds close events + keepOpenOnEscape, and adjusts rendering logic. |
| src/components/nav-drawer/nav-drawer.spec.ts | Updates a11y/DOM tests and adds behavior/event coverage for the native dialog implementation. |
- Render correctly on first update with initial open state - Correctly sync native dialog open state when changing drawer positioning
| /** | ||
| * Represents a side navigation container that provides | ||
| * quick access between views. | ||
| * `igc-nav-drawer` is a side navigation container that provides |
There was a problem hiding this comment.
Avoid component selector / class names in these doc comments if possible (maybe make that a repo skill note too), since those are picked up by other platforms. The old text was fine:
| * `igc-nav-drawer` is a side navigation container that provides | |
| * Represents a side navigation container that provides |
Or you can use the 'friendly' name like Navigation/Nav Drawer if you want to explicitly mention the name in the comment.
| * @csspart base - The base wrapper of the igc-navigation-drawer. | ||
| * @csspart main - The main container of the igc-navigation-drawer. | ||
| * @csspart mini - The mini container of the igc-navigation-drawer. | ||
| * @csspart base - The base wrapper of the drawer. A `<dialog>` element for non-relative positions, a `<div>` for relative. |
There was a problem hiding this comment.
Is it div or is it <nav> now? Also do we really need to go into so much detail? Not sure what styling scenarios the base is supposed to enable, but I'm not sure we should be that explicit about the internal structure (that's obv subject to change)
| - Added `keepOpenOnEscape` property — prevents the drawer from closing when the user presses the **Escape** key (non-relative positions only). | ||
| - Added `igcClosing` event — emitted just before the drawer is closed by user interaction. Cancelable. | ||
| - Added `igcClosed` event — emitted just after the drawer is closed by user interaction. | ||
| - The `mini` slot content is now always visible whenever it is provided, regardless of the drawer's position or open state. |
There was a problem hiding this comment.
Um, what? Not sure I'm getting this correctly, but is this changing the behavior of mini now? Besides being unrelated to the PR scope, it also doesn't make sense to me, mini is an alternative closed state.
…border directions - Make border declarations position-aware: inline borders for start/end, block borders for top/bottom - Group each position's rules into self-contained blocks - Consolidate backdrop lifecycle under &::backdrop / &:open::backdrop - Scope transition-delay for mini to the relative position context - Fix [part~='mini'] → [part='mini'] selector inconsistency - Update shared theme files to align with restructured base styles
…ut :open support - Add @supports not selector(:open) block providing host attribute-based open/close animations for browsers that don't yet support the :open pseudo-class (Safari < 26.5) - Override transition-property in the fallback to exclude overlay and display, preventing the closing glitch where the dialog falls to document flow and appears at the wrong position in browsers without overlay support
Description
Type of Change
Related Issues
Closes #2122, #2186, #2187
Checklist